[ENH] Recompress files where dcm2niix failed#803
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #803 +/- ##
==========================================
+ Coverage 83.24% 83.64% +0.40%
==========================================
Files 42 42
Lines 4404 4488 +84
==========================================
+ Hits 3666 3754 +88
+ Misses 738 734 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
octomike
force-pushed
the
fix/recompress_uncompressed_nii
branch
from
7aff494 to
a72d5ec
Compare
| "trying to salvage by recompressing ourselves. " | ||
| "This might take a while ") | ||
| if not recompress_failed(uncompressed): | ||
| raise RuntimeError("Error compressing nifti") |
There was a problem hiding this comment.
why to delay this crash here? this way you are hiding from user information on individual file and corresponding error -- why not just to just call recompress here and let error (if any happens) to bubble up?
I would also just loop here and have recompress to operate on a single file.
FWIW, I think it would be worth adding a test scenario to test this code path. I would have just mock patched the nipype_convert so that I would decompress some output file(s) and thus triggering this code path.
There was a problem hiding this comment.
Thanks, I'll look into it!
There was a problem hiding this comment.
Wow, completely forgot. Will definitely look again
There was a problem hiding this comment.
Took me just 6 months..
I addressed your suggestions and also added two test cases as you described (disclaimer: those were generated in a claude session - I don't know the policy here, but I'm open to redo those manually). The test cases check that recompress_failed actually recompresses the non-gz nifti returned from a mocked nipype_convert and also assert an error when loading a truncated/corrupt file because in the implementation I validate the uncompressed files are actually okay with nibabel.load().get_fdata().
octomike
force-pushed
the
fix/recompress_uncompressed_nii
branch
from
a72d5ec to
b133a4a
Compare
octomike
changed the title
octomike
force-pushed
the
fix/recompress_uncompressed_nii
branch
from
f360b96 to
22db16e
Compare
2 participants
As threatened in #802 an alternative approach.
I decided to leverage the fact that heudiconv blindly moves all converted output files to
nii.gz, even if they are justniiby renaming the successfully re-compressed image to.nii. This feels ugly, but results in the shortest/least-invasive PR.